Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maz + Kaliane Van #71

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open

Maz + Kaliane Van #71

wants to merge 43 commits into from

Conversation

mlweir98
Copy link

No description provided.

mlweir98 and others added 30 commits June 8, 2023 13:13
@@ -0,0 +1,140 @@
// `"use strict";`

const state = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love how you all chose to use this psuedostate! I think that it is great practice for what you will see in React though it will be utilized differently!

Comment on lines +22 to +38
const loadControls = () => {
state.increaseTempButton = document.getElementById('increaseTempButton');
state.decreaseTempButton = document.getElementById('decreaseTempButton');
state.tempNumber = parseInt(document.getElementById('tempNumberContainer').innerText);
state.tempNumberContainer = document.getElementById('tempNumberContainer');
state.tempNumberClass = document.getElementById('tempNumberContainer').className;
state.skyEmojiContainer = document.getElementById('skyEmojiContainer');
state.skyEmoji = document.getElementById('skyEmojiContainer').innerText;
state.landEmojiContainer = document.getElementById('landEmojiContainer');
state.landEmoji = document.getElementById('landEmojiContainer').innerText;
state.cityNameContainer = document.getElementById('cityNameContainer');
state.cityName = document.getElementById('cityNameContainer').innerText;
state.cityInput = document.getElementById('cityInput');
state.realTempButton = document.getElementById('realTempButton');
state.skyDropdown = document.getElementById('skyDropdown');
state.weatherCity = document.getElementById('weatherCity');
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow! I love this! This shows me you centering readability! Naming conventions like this make your code a lot more intuitive!

Comment on lines +83 to +92
const getRealTemp = async (locationName) => {
const locationResp = await axios.get('http://localhost:5000/location',{ params: {q: locationName } })

const coords = [locationResp.data[0]['lat'], locationResp.data[0]['lon']]

const weatherResponse = await getWeather(coords[0],coords[1])

return weatherResponse

}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay async/await! You used this perfectly!

state.tempNumber = response.data['main']['temp']
};

const registerEventHandlers = () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⭐️⭐️⭐️

registerEventHandlers();
};

onLoaded();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest putting your onLoaded method inside an event handler for when the document is loaded, that way you can have an extra layer of protection when it comes to things loading in order. Basically, it would help ensure the loadControls and. registerEventHandlers functions would run after the DOM elements are loaded. It would look something like this:

document.addEventListener("DOMContentLoaded", function () {
    loadControls()
    registerEventHandlers()
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job everyone! You really ate this one up! Please feel free to reach out to me if you have any questions about the comments I left or if you want to discuss anything in greater detail! ⭐️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants